Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DONE] add i18n dynamic url #1135

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

[DONE] add i18n dynamic url #1135

wants to merge 3 commits into from

Conversation

CartierPierre
Copy link
Contributor

In reference of #1130

Tested only on channels, it have to be tested on other impacted URL (video, videos, playlists)

There is no sense to use i18n for hidden or not shared urls

@Badatos
Copy link
Collaborator

Badatos commented Oct 8, 2024

Cette PR est marquée [WIP], le travail semble donc toujours en cours ?
Quand une PR est prête pour la revue, merci de la marquer en "[DONE]".

@CartierPierre
Copy link
Contributor Author

En soit, c'est codé, je n'ai juste pas pu tout tester, j'ai seulement testé sur les chaines, mais pas sur les vidéos

@Badatos
Copy link
Collaborator

Badatos commented Oct 8, 2024

En soit, c'est codé, je n'ai juste pas pu tout tester, j'ai seulement testé sur les chaines, mais pas sur les vidéos

dans ce cas, tu peux renommer le [WIP] en [DONE] ?
À voir si on trouve une bonne âme pour tester cela du coup...

@Badatos
Copy link
Collaborator

Badatos commented Oct 8, 2024

Au passage, peux-tu mettre à jour ta branche avec la dernière version de [develop] ?

@CartierPierre CartierPierre changed the title [WIP] add i18n dynamic url [DONE] add i18n dynamic url Oct 9, 2024
@CartierPierre
Copy link
Contributor Author

@Badatos Voilà 😃

@Badatos
Copy link
Collaborator

Badatos commented Oct 9, 2024 •
edited
Loading

Merci Pierre ;)

Visiblement, il va falloir mettre a jour les tests unitaires pour que cette modif passe.

Il y a par exemple "test_get_channels_for_specific_channel_tab" qui teste /second-channel/ et recoit /en/second-channel/

cf https://github.com/EsupPortail/Esup-Pod/actions/runs/11250448930/job/31279336593?pr=1135 pour le log complet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gcondess gcondess gcondess approved these changes

Assignees

@CartierPierre CartierPierre

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CartierPierre @Badatos @gcondess